hi Julien, this is a nice re-work, but it will need a lot of testing on different boards. I did a spot-check on a Pixhawk1 and found the 2nd accel ends up with the wrong rotation. The PR changes the LSM9DS0 driver gyro orientation but not the accel orientation. I think others are wrong too.

I just checked master and it looks like we have an existing error in the l3gd20 gyro orientation from my sensor re-work. I'll work on fixing that now.

i've fixed the l3gd20 rotation, and removed the raspilot hack in the driver. It turns out we need a different rotation for the gyro than for the accel. I had assumed they were the same in my recent changes.
I'd like to get the rest of your changes in, we just need to ensure it keeps the rotations right.

@tridge I don't think that it is possible that we need a different rotation for the accels and the gyros.
Ok. I'll rework that.

fixed the rotation for accel in L3G and LSM drivers.

Rebased. Tested OK on pixhawk.
So tested on Bebop Disco and Pixhawk.

I've tested pixhawk2, pixracer, pixfalcon, phmini raspilot. There were a number of fixes needed:
https://github.com/tridge/ardupilot/commits/pr-julien-orient
It looks to me like others are wrong too. I don't think we can push this in yet. We would need a more systematic way of getting these right.

Sorry for wasting your time. They are leftovers and a mistake for raspilot. My assumptions regarding rotation changes in the PR message are right.

@tridge I added your corrections to my commit for clarity purpose. If you prefer, I could just push them on my pr branch with the previous version.

I don't mind either way, just need to do more testing :-)

@lucasdemarchi You said you could test that PR with some boards. I am waiting some more tests before rebasing. Would you have time for that?

@jberaud Is this something you'd still like to work on? It needs a big rebase and I think that if you aren't able to rework it there is no point in keeping it open.

I can rebase again, but since there are some boards that I don't have, I won't be anymore able to test this PRs on them. So there are 3 solutions :
-> Someone who has these boards tests
-> Someone sends me those boards
-> You can close it. It is not such a big change but it needs testing...

I think this is a very worthwhile PR and from comments above I think @tridge does too. I think we can get people to test the boards - coming up with a test procedure for people to follow might help with that.

Hi, sorry for the long delay. We do need to test it, but I think getting it mathematically correct would allow us to be more confident on applying it even if we can't test it everywhere.

    x = y, y = x, z = -z
    meaning ROTATION_ROLL_180_YAW_90

This actually means ROTATION_ROLL_180_YAW_270 since it's and intrinsic xyz rotation. You can also check on Vector3::rotate() that what I just said matches the code. Manually checking some of them:

PXF: roll180, yaw270, roll180, yaw270 = yaw180 -> correct on patch
PXFMini: roll180, yaw270, roll180, yaw90 = none -> correct on patch
Bebop: roll180, yaw270, yaw270 = pitch180 -> it's roll180 on patch
Disco: roll180, yaw270, pitch180, yaw90 = yaw180 -> correct on patch
...

The checks above were made manually. I think I had at some point a script using sympy to work out the rotations. If I don't find it I'll derive the rotations again so we can be more confident they are correct.

@lucasdemarchi Thanks very much for your inputs. Since I have tested that on Bebop, there must be something in my params that I was assuming wrongly. I'll check that, fix the patch, and rebase. Thanks again.

@lucasdemarchi I have checked the code in vector3 and here is what I see
case ROTATION_ROLL_180_YAW_90: { tmp = x; x = y; y = tmp; z = -z; return; }

Could you explain why it should be ROTATION_ROLL_180_YAW_270 ?

Could you explain why it should be ROTATION_ROLL_180_YAW_270

Oh... I always forget the convention we are using... It's zyx, not xyz, i.e. yaw, pitch and roll in order. We need to add a comment on rotations.{h,CPP}. Now to check the others places

@jberaud I rechecked all the rotations you changed with the following test program: https://gist.github.com/d049489154f4f90141644b3c90403009

All of them are correct. Could you rebase this?

@lucasdemarchi Done.

i'm quite nervous about this patch - it is very hard to validate it fully. We thought it was right before and it turned out to be wrong.

@jberaud @lucasdemarchi Can we come up with some steps to test this PR? I can bother people to test boards, but without a testing procedure, it is hard.

@OXINARF Step tests should consist in determining if the orientation of the Accel and Gyro are correct.
So first thing would be to do an accel calibration.

Then to check the gyro orientation, graph gyro (ggyro in mavproxy with mavinit.scr script for primary gyro or ggyro2 for secondary gyro).
Then take the board in hands facing you, meaning what you can see is the front of the board (X axis).
When turning it clockwise, the X line should go negative.

Then take the board in your hand facing its right side (Y axis). Turn it clockwise and the Y axis should go negative.

Then take the board in your hand facing its bottom (Z axis). Turn it clockwise and the Z axis should go negative.

Then to check the accel orientation, graph accel (gaccel for primary and gaccel2 for secondary, etc...).
When the board is on the table (up up and bottom down), Z axis should be negative.

Then turn it and then, put it on its right side (Y axis facing the table). Y should be negative.

Then turn it, put it nose down (X axis facing the table). X should become negative.

Note: When I say "Right" side, I am in the regular drone coordinate system, meaning drone facing straight ahead, not pointing towards you.

@tridge I understand your concerns. I have seen master being broken numerous times on Bebop but I guess it is because it is quite an uncommon board for ardupilot. I hope we can test this PR correctly enough to merge it, because in the end, it is all about testing right.